[PATCH 08/11] apparmor: fix unprivileged local user can do privileged policy management

An unprivileged local user can load, replace, and remove profiles by
opening the apparmorfs interfaces, via a confused deputy attack, by
passing the opened fd to a privileged process, and getting the
privileged process to write to the interface.

This does require a privileged target that can be manipulated to do
the write for the unprivileged process, but once such access is
achieved full policy management is possible and all the possible
implications that implies: removing confinement, DoS of system or
target applications by denying all execution, by-passing the
unprivileged user namespace restriction, to exploiting kernel bugs for
a local privilege escalation.

The policy management interface can not have its permissions simply
changed from 0666 to 0600 because non-root processes need to be able
to load policy to different policy namespaces.

Instead ensure the task writing the interface has privileges that
are a subset of the task that opened the interface. This is already
done via policy for confined processes, but unconfined can delegate
access to the opened fd, by-passing the usual policy check.

Fixes: c88d4c7b049e8 ("AppArmor: core policy routines")
Reported-by: Qualys Security Advisory <qsa@qualys.com>
Tested-by: Salvatore Bonaccorso <carnil@debian.org>
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Reviewed-by: Cengiz Can <cengiz.can@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Gbp-Pq: Topic bugfix/all/qsa-2026-apparmor
Gbp-Pq: Name 0008-apparmor-fix-unprivileged-local-user-can-do-privileg.patch

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 9252172d50682b6aba9f535fee6ff35e4702f85c..9ba41fe273208762901a7b7c0cf557e8cf58da34 100644 (file)
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -417,7 +417,8 @@ static struct aa_loaddata *aa_simple_write_to_buffer(const char __user *userbuf,
 }
 
 static ssize_t policy_update(u32 mask, const char __user *buf, size_t size,
-                            loff_t *pos, struct aa_ns *ns)
+                            loff_t *pos, struct aa_ns *ns,
+                            const struct cred *ocred)
 {
        struct aa_loaddata *data;
        struct aa_label *label;
@@ -428,7 +429,7 @@ static ssize_t policy_update(u32 mask, const char __user *buf, size_t size,
        /* high level check about policy management - fine grained in
         * below after unpack
         */
-       error = aa_may_manage_policy(current_cred(), label, ns, mask);
+       error = aa_may_manage_policy(current_cred(), label, ns, ocred, mask);
        if (error)
                goto end_section;
 
@@ -449,7 +450,8 @@ static ssize_t profile_load(struct file *f, const char __user *buf, size_t size,
                            loff_t *pos)
 {
        struct aa_ns *ns = aa_get_ns(f->f_inode->i_private);
-       int error = policy_update(AA_MAY_LOAD_POLICY, buf, size, pos, ns);
+       int error = policy_update(AA_MAY_LOAD_POLICY, buf, size, pos, ns,
+                                 f->f_cred);
 
        aa_put_ns(ns);
 
@@ -467,7 +469,7 @@ static ssize_t profile_replace(struct file *f, const char __user *buf,
 {
        struct aa_ns *ns = aa_get_ns(f->f_inode->i_private);
        int error = policy_update(AA_MAY_LOAD_POLICY | AA_MAY_REPLACE_POLICY,
-                                 buf, size, pos, ns);
+                                 buf, size, pos, ns, f->f_cred);
        aa_put_ns(ns);
 
        return error;
@@ -492,7 +494,7 @@ static ssize_t profile_remove(struct file *f, const char __user *buf,
         * below after unpack
         */
        error = aa_may_manage_policy(current_cred(), label, ns,
-                                    AA_MAY_REMOVE_POLICY);
+                                    f->f_cred, AA_MAY_REMOVE_POLICY);
        if (error)
                goto out;
 
@@ -1826,7 +1828,7 @@ static struct dentry *ns_mkdir_op(struct mnt_idmap *idmap, struct inode *dir,
        int error;
 
        label = begin_current_label_crit_section();
-       error = aa_may_manage_policy(current_cred(), label, NULL,
+       error = aa_may_manage_policy(current_cred(), label, NULL, NULL,
                                     AA_MAY_LOAD_POLICY);
        end_current_label_crit_section(label);
        if (error)
@@ -1876,7 +1878,7 @@ static int ns_rmdir_op(struct inode *dir, struct dentry *dentry)
        int error;
 
        label = begin_current_label_crit_section();
-       error = aa_may_manage_policy(current_cred(), label, NULL,
+       error = aa_may_manage_policy(current_cred(), label, NULL, NULL,
                                     AA_MAY_LOAD_POLICY);
        end_current_label_crit_section(label);
        if (error)

diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h
index 4c50875c9d13e814f3d98be9ece20a596cbfda01..a37b159516b002c59f081b5ae2fa84c1c1f242f2 100644 (file)
--- a/security/apparmor/include/policy.h
+++ b/security/apparmor/include/policy.h
@@ -419,7 +419,7 @@ bool aa_policy_admin_capable(const struct cred *subj_cred,
                             struct aa_label *label, struct aa_ns *ns);
 int aa_may_manage_policy(const struct cred *subj_cred,
                         struct aa_label *label, struct aa_ns *ns,
-                        u32 mask);
+                        const struct cred *ocred, u32 mask);
 bool aa_current_policy_view_capable(struct aa_ns *ns);
 bool aa_current_policy_admin_capable(struct aa_ns *ns);
 

diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index f0e554f193550d9ddf5cf8047947ad15aabaec04..3623ec620cba4a662726e0e717895cacbcb31e0e 100644 (file)
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -925,17 +925,44 @@ bool aa_current_policy_admin_capable(struct aa_ns *ns)
        return res;
 }
 
+static bool is_subset_of_obj_privilege(const struct cred *cred,
+                                      struct aa_label *label,
+                                      const struct cred *ocred)
+{
+       if (cred == ocred)
+               return true;
+
+       if (!aa_label_is_subset(label, cred_label(ocred)))
+               return false;
+       /* don't allow crossing userns for now */
+       if (cred->user_ns != ocred->user_ns)
+               return false;
+       if (!cap_issubset(cred->cap_inheritable, ocred->cap_inheritable))
+               return false;
+       if (!cap_issubset(cred->cap_permitted, ocred->cap_permitted))
+               return false;
+       if (!cap_issubset(cred->cap_effective, ocred->cap_effective))
+               return false;
+       if (!cap_issubset(cred->cap_bset, ocred->cap_bset))
+               return false;
+       if (!cap_issubset(cred->cap_ambient, ocred->cap_ambient))
+               return false;
+       return true;
+}
+
+
 /**
  * aa_may_manage_policy - can the current task manage policy
  * @subj_cred: subjects cred
  * @label: label to check if it can manage policy
  * @ns: namespace being managed by @label (may be NULL if @label's ns)
+ * @ocred: object cred if request is coming from an open object
  * @mask: contains the policy manipulation operation being done
  *
  * Returns: 0 if the task is allowed to manipulate policy else error
  */
 int aa_may_manage_policy(const struct cred *subj_cred, struct aa_label *label,
-                        struct aa_ns *ns, u32 mask)
+                        struct aa_ns *ns, const struct cred *ocred, u32 mask)
 {
        const char *op;
 
@@ -951,6 +978,11 @@ int aa_may_manage_policy(const struct cred *subj_cred, struct aa_label *label,
                return audit_policy(label, op, NULL, NULL, "policy_locked",
                                    -EACCES);
 
+       if (ocred && !is_subset_of_obj_privilege(subj_cred, label, ocred))
+               return audit_policy(label, op, NULL, NULL,
+                                   "not privileged for target profile",
+                                   -EACCES);
+
        if (!aa_policy_admin_capable(subj_cred, label, ns))
                return audit_policy(label, op, NULL, NULL, "not policy admin",
                                    -EACCES);